Why not first

let url = Url::parse(p).map_err(...)?;
// then
match url {
...
}

the current match combines all checks under it. It is an error also when some insecure scheme is used.

Ok. A first impression is

// failed to parse
let url = Url::parse(p).map_err(...)?;

if url.scheme() == "https" {
    ...
}

if url.scheme() == "file" && Path::new(url.path()).exists() {
    ...
}

...

I can move the error check arm to be the first one?

moved

I second Ding's original suggestion here. I would just get the error check out of the way in the beginning rather than adding another set of brackets. This will also allow you to return a more specific error.

I think it tends to be more readable to use match statements more like switch statements and to avoid chaining them together unless it really fits the fundamental logic of the function.

I just updated this part too, thanks for the feedback!

I second Ding's original suggestion here. I would just get the error check out of the way in the beginning rather than adding another set of brackets. This will also allow you to return a more specific error.

I think it tends to be more readable to use match statements more like switch statements and to avoid chaining them together unless it really fits the fundamental logic of the function.

I wouldn't use a match here since there is really no logic in this case. You can just use map instead.

Agreed. With the type change of trusted_certs_paths, we can simply write as

    pub async fn new(config: AttestationTokenVerifierConfig) -> Result<Self> {
        let keys = stream::iter(config.trusted_certs_paths)
            .filter_map(|p| async move {
                match get_jwks_from_file_or_url(&p).await {
                    Ok(jwkset) => Some(jwkset.keys),
                    Err(e) => {
                        warn!("error getting JWKS: {e:?}");
                        None
                    }
                }
            })
            .concat()
            .await;

        Ok(Self { trusted_certs: jwk::JwkSet { keys } })
    }

where futures_util crate is needed.

Right, I guess I blindly followed the CoCo approach so it ended up looking the same

where futures_util crate is needed.

the proposal looks nice but I wonder if this is justified for the functionality here?

What the stream::iter does, is try to do get_jwks_from_file_or_url concurrently rather than serially if multiple paths in trusted_certs_paths are given. This makes sense to me. wdyt?

My use-case is with one URL only but I understand we may have several trusted_certs_path in the future where parallelism might help to improve the KBS startup time.

Agreed. With the type change of trusted_certs_paths, we can simply write as

    pub async fn new(config: AttestationTokenVerifierConfig) -> Result<Self> {
        let keys = stream::iter(config.trusted_certs_paths)
            .filter_map(|p| async move {
                match get_jwks_from_file_or_url(&p).await {
                    Ok(jwkset) => Some(jwkset.keys),
                    Err(e) => {
                        warn!("error getting JWKS: {e:?}");
                        None
                    }
                }
            })
            .concat()
            .await;

        Ok(Self { trusted_certs: jwk::JwkSet { keys } })
    }

where futures_util crate is needed.